Skip to content

(FM-8643) Replace requires with feature confine #266

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

(FM-8643) Replace requires with feature confine #266

wants to merge 3 commits into from

Conversation

@michaeltlombardi
Copy link
Contributor

@michaeltlombardi michaeltlombardi commented Oct 24, 2019

Prior to this commit the providers loaded the ruby-pwsh library
via a requires statement. When the powershell module is installed
without the dependency module, pwshlib, this requires statement
blows up as the library cannot be found.

This commit adds a ruby-pwsh feature and leverages it for the
providers.

@michaeltlombardi
(FM-8643) Replace requires with feature confine
3459e43 
Prior to this commit the providers loaded the ruby-pwsh library
via a requires statement. When the powershell module is installed
without the dependency module, pwshlib, this requires statement
blows up as the library cannot be found.

This commit adds a ruby-pwsh feature and leverages it for the
providers.
@michaeltlombardi
Copy link
Contributor Author

michaeltlombardi commented Oct 29, 2019

@puppetlabs/modules I've updated this PR with a second implementation option. Some notes:

  • To see the various implementations you'll have to review on a commit-by-commit basis.
  • There really isn't a good way to generalize this pattern across modules, since the problem is we can't depend on, well, dependencies being installed
  • We could make this less of a hassle for future module authors with a PDK template or helpers of some sort, perhaps
  • If we feature confines we cannot recommend using the library until the resource API supports them
  • This explosion is only possible if modules are installed without regard for dependency chains. Should we instead let the run explode but with an explicit and clear error message that the pwshlib module is not installed in the environment?

@michaeltlombardi
(MAINT) Alternate implementation
8e449d2 
This commit includes an alternate to relying on the
confine function which is not available in modules
build on the resource api.
@michaeltlombardi
Copy link
Contributor Author

michaeltlombardi commented Oct 29, 2019

And the third way, with an explicit failure pointing you at the right thing to do (commit incoming, had to blow away and rebuild bundle, need to test):

image

@michaeltlombardi
(MAINT) Fail loudly if dependency missing
008a5e9 
This commit causes the loading of the module to fail
loudly and clearly if the dependency module is not
found.
@glennsarti
Copy link
Contributor

glennsarti commented Oct 30, 2019

Does this work in a master-agent situation?

@michaeltlombardi
Copy link
Contributor Author

michaeltlombardi commented Oct 30, 2019

Closed in favor of #267

@michaeltlombardi michaeltlombardi deleted the ticket/master/FM-8643-feature-confine branch October 29, 2020 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@michaeltlombardi @glennsarti